Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjustments to support Coq 8.18.0 #1763

Merged
merged 4 commits into from
Sep 18, 2023
Merged

Conversation

jdchristensen
Copy link
Collaborator

Allow Pushout_ind (and therefore various other things) to compute; loosen some universe variables; change Let ... Qed to Local Definition ... Qed to truly hide the contents (and make some proof terms much smaller); silence two warnings about minimization to Set.

Each commit can be read independently.

Everything builds fine with Coq 8.17 as well.

@Alizter
Copy link
Collaborator

Alizter commented Sep 18, 2023

I like your version better since it gets rid of a dubious technique with the opaque lets. Coq added proper support for that use case with the new #[clearbody] attribute, but obviously that make 8.17 incompat.

@jdchristensen
Copy link
Collaborator Author

Thanks, I'll merge. In general, I'm going to lean towards faster merging for straightforward changes, since it's getting harder to get two approvals. I'm always happy to make further changes after merging if anyone notices things that can be improved.

@jdchristensen jdchristensen merged commit 4e9183a into HoTT:master Sep 18, 2023
23 checks passed
@jdchristensen jdchristensen deleted the 8.18-fixes branch September 18, 2023 17:50
@Alizter
Copy link
Collaborator

Alizter commented Sep 18, 2023

@jdchristensen I think the 2 approvals requirement is a bit hopeful for the time being. I don't mind asking for 1 as long as we continue to review like we have been doing.

@mikeshulman
Copy link
Contributor

Yeah, if we really only have 2 active contributors, asking for 2 approvals on each PR is a bit much. (-:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants